Skip to content

S_invlist_trim - don't SvPV_renew if SvLEN(invlist) is < PTRSIZE larger #23111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: blead
Choose a base branch
from
Open

S_invlist_trim - don't SvPV_renew if SvLEN(invlist) is < PTRSIZE larger #23111

wants to merge 1 commit into from

Conversation

richardleach
Copy link
Contributor

Currently, S_invlist_trim always calls SvPV_renew(invlist, <size>),
which is a macro wrapping a call to safesysrealloc().

However, SvLEN(invlist) is often already exactly the desired size,
or it is larger by less than the size of a pointer. With this commit,
such cases just return without calling SvPV_renew().

  • This set of changes does not require a perldelta entry.

@richardleach
Copy link
Contributor Author

Tow comments:

  • It will still be common to try to resize SvLEN == 34 down to 25 bytes. Given that it's common for malloc implementations to increase chunk sizes by 2 x PTRLEN, should that be the check here too?
  • Allocated chunks will be pointer-aligned. 34 is not a multiple of 8! Maybe we should round up allocation sizes to a multiple of the pointer size generally?

@richardleach richardleach linked an issue Mar 17, 2025 that may be closed by this pull request
S_invlist_trim maybe doesn't need to SvPV_renew every time? #23103
Open
@richardleach richardleach mentioned this pull request Apr 15, 2025
Open
@bulk88
Copy link
Contributor

bulk88 commented Apr 18, 2025

S_invlist_trim_pv_shrink_evt_trace.txt
made a log, "if( new == old ) goto skip;" isn't captured in the log. I already removed that permutation in the test build. byte counts are in hex/base16 on Win64 Perl, and the byte counts are whatever is in SvLEN field before and after.

There are so many problems with this API, IDK where to start. For example, the same SV* head is repeatedly getting shrunk. So why isn't perl core keeping it at the max/peak length? That means a single INVLIST GC pass at the "end" (IDK where "end" is), triggered from C stack C fn scope, or triggered by a save stack entry (runloop).

Also "new 0x9" which is (UV)0+'\0' probably shouldn't even exist in the SVt_INVLIST API on a C level, since that means "empty" or "0 entries", since the first sizeof(UV) bytes, at the start of the SvPVX in a SVt_INVLIST is always a items counter.

Suggestion, why not just turn off SvPOK or its analog, Safefree() the SvPVX buffer, and set SvLEN to 0 inside a SVt_INVLIST SV* obj instead of keep a Newx that just means undef or zero on a high[-er] [API] level anyway?

ln 50 $args{strip_suffix} = $OPT{'s'} =~ m#^/([^/]*[^/\$]|)\$?/?$# ? qr/$1/ : qr/\Q$OPT{'s'}\E/;

S_invlist_trim old 42 new 19 at 1d7ec10 at C:\sources\perl5\win32\bin\pl2bat.pl line 50.
S_invlist_trim old 42 new 19 at 1d7ec10 at C:\sources\perl5\win32\bin\pl2bat.pl line 50.
S_invlist_trim old 42 new 19 at 1d7ec10 at C:\sources\perl5\win32\bin\pl2bat.pl line 50.
S_invlist_trim old 42 new 19 at 1d7ec10 at C:\sources\perl5\win32\bin\pl2bat.pl line 50.
S_invlist_trim old 42 new 19 at 1d7ec10 at C:\sources\perl5\win32\bin\pl2bat.pl line 50.
S_invlist_trim old 42 new 19 at 1d7ec10 at C:\sources\perl5\win32\bin\pl2bat.pl line 50.
S_invlist_trim old 42 new 19 at 1d7ec10 at C:\sources\perl5\win32\bin\pl2bat.pl line 50.

ln 195 my (@rest) = ($args =~ /([^\s:])(?!\s*:)/g);

S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old aa new 9 at 1d7ed78 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.
S_invlist_trim old ba new b9 at 1d7ee20 at C:\sources\perl5\lib/Getopt/Std.pm line 195.

ln 51 @e = split(/((?:\PM\pM*|^\pM+){$tabstop})/,$line,-1);
$tabstop is a spiritual const at runtime, set 1x per proc as our $tabstop = 8;.

S_invlist_trim old 1382 new 19 at 2551f60 at C:\sources\perl5\cpan\Text-Tabs\lib/Text/Tabs.pm line 51, <$fh> line 668.
S_invlist_trim old 1382 new 19 at 2551fa8 at C:\sources\perl5\cpan\Text-Tabs\lib/Text/Tabs.pm line 51, <$fh> line 668.
S_invlist_trim old 1382 new 19 at 2551fa8 at C:\sources\perl5\cpan\Text-Tabs\lib/Text/Tabs.pm line 51, <$fh> line 668.
S_invlist_trim old 1382 new 19 at 2551fa8 at C:\sources\perl5\cpan\Text-Tabs\lib/Text/Tabs.pm line 51, <$fh> line 668.
S_invlist_trim old 1382 new 19 at 2551fa8 at C:\sources\perl5\cpan\Text-Tabs\lib/Text/Tabs.pm line 51, <$fh> line 668.
S_invlist_trim old 1382 new 19 at 2551fa8 at C:\sources\perl5\cpan\Text-Tabs\lib/Text/Tabs.pm line 51, <$fh> line 668.
S_invlist_trim old 1382 new 19 at 2551fa8 at C:\sources\perl5\cpan\Text-Tabs\lib/Text/Tabs.pm line 51, <$fh> line 668.
S_invlist_trim old 1382 new 19 at 2551fa8 at C:\sources\perl5\cpan\Text-Tabs\lib/Text/Tabs.pm line 51, <$fh> line 668.
S_invlist_trim old 1382 new 19 at 2551fa8 at C:\sources\perl5\cpan\Text-Tabs\lib/Text/Tabs.pm line 51, <$fh> line 668.
S_invlist_trim old 1382 new 19 at 2551fa8 at C:\sources\perl5\cpan\Text-Tabs\lib/Text/Tabs.pm line 51, <$fh> line 668.

ln 1425 return undef unless my ($quote_type, $value) = $Config_SH_expanded =~ /\n$key=(['"])(.*?)\1\n/s;
This one looks like a total failure to precompute data at BEGIN phase/PP Compile time. SV* 0x22c90d0 and the others are repeating in a pattern.

S_invlist_trim old 3a new 9 at 22c90d0 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4259.
S_invlist_trim old 2a new 9 at 22c9130 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4259.
S_invlist_trim old 22 new 19 at 22ca658 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4259.
S_invlist_trim old 3a new 9 at 22c2fc8 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4260.
S_invlist_trim old 2a new 9 at 22c90e8 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4260.
S_invlist_trim old 22 new 19 at 22ca568 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4260.
S_invlist_trim old 3a new 9 at 22c0c48 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4261.
S_invlist_trim old 2a new 9 at 22c0cf0 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4261.
S_invlist_trim old 22 new 19 at 22ca598 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4261.
S_invlist_trim old 3a new 9 at 22ca628 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4262.
S_invlist_trim old 2a new 9 at 22ca658 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4262.
S_invlist_trim old 22 new 19 at 22c3010 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4262.
S_invlist_trim old 3a new 9 at 22c9130 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4263.
S_invlist_trim old 2a new 9 at 22ca568 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4263.
S_invlist_trim old 22 new 19 at 22c90d0 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4263.
S_invlist_trim old 3a new 9 at 22c90e8 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4272.
S_invlist_trim old 2a new 9 at 22ca598 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4272.
S_invlist_trim old 22 new 19 at 22c2fc8 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4272.
S_invlist_trim old 3a new 9 at 22c0cf0 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4281.
S_invlist_trim old 2a new 9 at 22c3010 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4281.
S_invlist_trim old 22 new 19 at 22c0c48 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4281.
S_invlist_trim old 3a new 9 at 22ca658 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4282.
S_invlist_trim old 2a new 9 at 22c90d0 at C:\sources\perl5\lib/Config_heavy.pl line 1425, <SH> line 4282.

Here is a sample of the different kinds of actual sizes/SvLEN values encountered by this API during a gmake.exe all.

S_invlist_trim old 304a new 69 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pm line 183.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1399.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 87.
S_invlist_trim old 30a2 new 2fe9 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pm line 183.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1403.
S_invlist_trim old 3052 new 69 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pm line 183.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1403.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 87.
S_invlist_trim old 304a new 3039 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pm line 183.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1403.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 87.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1403.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 87.
S_invlist_trim old ba new 29 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old d2 new 89 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old b2 new 29 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old 9a new 9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old 22 new 19 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pm line 349.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1403.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old 22 new 19 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pm line 350.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1436.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.ources\perl5\lib\CORE\zaphod32_hash.h
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old 3a new 9 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pmline 353.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1437.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 88.
S_invlist_trim old ba new 29 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old 2a new 9 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pmline 353.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1487.
S_invlist_trim old d2 new 89 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old 22 new 19 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pm line 356.
S_invlist_trim old 2a new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1487.
S_invlist_trim old b2 new 29 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old 32 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1487.
S_invlist_trim old 3a new 9 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pmline 572.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1487.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old 2a new 9 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pmline 572.
S_invlist_trim old 32 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1487.
S_invlist_trim old 9a new 9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old 2a new 9 at C:\sources\perl5\cpan\File-Path\lib/File/Path.pmline 572.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old 304a new 69 at C:\sources\perl5\lib/Config_heavy.pl line 1493.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 98.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 42 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 3a new 11 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old ba new 9 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old aa new 9 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 3a new 29 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 3a new 9 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 115.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 3052 new 69 at C:\sources\perl5\lib/Config_heavy.pl line 1493.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 304a new 69 at C:\sources\perl5\lib/Config_heavy.pl line 1496.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 30a2 new 2fe9 at C:\sources\perl5\lib/Config_heavy.pl line 1496.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 3052 new 69 at C:\sources\perl5\lib/Config_heavy.pl line 1496.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 304a new 3039 at C:\sources\perl5\lib/Config_heavy.pl line 1496.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 10.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 14.
S_invlist_trim old 22 new 19 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old 22 new 9 at ../regen/HeaderParser.pm line 93.
S_invlist_trim old ba new 29 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 32.
S_invlist_trim old d2 new 89 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old b2 new 29 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 9a new 9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 49.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 304a new 69 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 30a2 new 2fe9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 3052 new 69 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 304a new 3039 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 304a new 69 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 30a2 new 2fe9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 3052 new 69 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 304a new 3039 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old ba new 29 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old d2 new 89 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old b2 new 29 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 9a new 9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old ba new 29 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old d2 new 89 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old b2 new 29 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 9a new 9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 304a new 69 at C:\sources\perl5\lib/Config_heavy.pl line 157.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 304a new 69 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 30a2 new 2fe9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 3052 new 69 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old 30a2 new 2fe9 at C:\sources\perl5\lib/Config_heavy.pl line 157.
S_invlist_trim old 304a new 3039 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old 3052 new 69 at C:\sources\perl5\lib/Config_heavy.pl line 157.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old 304a new 3039 at C:\sources\perl5\lib/Config_heavy.pl line 157.
S_invlist_trim old ba new 29 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 157.
S_invlist_trim old d2 new 89 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old b2 new 29 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old 9a new 9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1386.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1386.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1386.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1399.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1399.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1399.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1399.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 22 new 19 at C:\sources\perl5\lib/Config_heavy.pl line 1399.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.
S_invlist_trim old aa new a9 at ../regen/HeaderParser.pm line 116.
S_invlist_trim old 3a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1403.
S_invlist_trim old 2a new 9 at C:\sources\perl5\lib/Config_heavy.pl line 1425.

This was referenced May 15, 2025
Closed
Merged
@bulk88
Copy link
Contributor

bulk88 commented Jun 8, 2025

I'm just gonna drop a link here in this randomly picked ticket #23359

There are around 6 - 10 stalled PRs open related to the regexp engine's abuse of RenewTrim() and SvPV_shrink_to_cur(sv).

And another about 8-12 bug reports without code/patches/fixes open right now related to the regexp engine's abuse of RenewTrim() and SvPV_shrink_to_cur(sv).

And 2-6 bug reports without code/patches/fixes open right now related to the <>/Perl_sv_gets();s abuse of RenewTrim() and SvPV_shrink_to_cur(sv).

@richardleach richardleach added the do not merge Don't merge this PR, at least for now label Jun 18, 2025
@richardleach richardleach force-pushed the S_invlist_trim-notsomuch branch from bd9b4e9 to 80dd43e Compare August 9, 2025 21:57
@richardleach
Copy link
Contributor Author

I've modified this PR to make use of the recently introduced Perl_expected_size macro.

The current code does still see SvPV_renew called needlessly - for example when SvLEN ==40, the desired size is 32, but the valid allocation sizes are 24 bytes and 40 bytes. Improvements to come to Perl_expected_size seem the better solution to that sort of case rather than applying fudge factors inside S_invlist_trim.

Ready for review.

@richardleach richardleach removed the do not merge Don't merge this PR, at least for now label Aug 9, 2025
@richardleach
Copy link
Contributor Author

Though if there's consensus that the remaining amount of reallocation for small gain is undesirable, I'm not opposed to re-tweaking the condition from:

    if ( SvLEN(invlist) > realistic_size ) {

to something like:

    if ( SvLEN(invlist) > realistic_size + PTRSIZE) {

khwilliamson
khwilliamson previously requested changes Aug 10, 2025
View reviewed changes
regcomp_invlist.c Outdated
* the malloc implementation in use. */

const STRLEN min_size = (PERL_STRLEN_NEW_MIN > TO_INTERNAL_SIZE(1) + 1)
? PERL_STRLEN_NEW_MIN : TO_INTERNAL_SIZE(1) + 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use MAX() here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, thanks. I'll do that.

@khwilliamson
Copy link
Contributor

How many bytes do we consider too little to bother with trimming? I don't know. But I'd like to see some ideas. I'd be fine if others are willing to tolerate some excess that doesn't get trimmed in the service of fewer malloc calls

@richardleach
Copy link
Contributor Author

How many bytes do we consider too little to bother with trimming? I don't know. But I'd like to see some ideas. I'd be fine if others are willing to tolerate some excess that doesn't get trimmed in the service of fewer malloc calls

I don't have a good understanding of the lifetime of invlists. Do some hang around under interpreter destruction but others might be short lived? (Less shrinkage seems like a reasonable trade off for the latter, but less so for the former.)

@richardleach
S_invlist_trim - don't SvPV_renew where no change is likely
573e4fd 
Currently, `S_invlist_trim` always calls `SvPV_renew(invlist, <size>)`,
which is a macro wrapping a call to `safesysrealloc()`.

However, `SvLEN(invlist)` is often already exactly the desired size, or
it is larger by less than the size of a pointer. With this commit, the
new `expected_size` macro is used to reduce the number of cases in which
S_invlist_trim will try to shrink a buffer but no shrinkage is likely to
occur. (For example, if the desired size is less than the minimum actual
allocation size.)
@richardleach richardleach force-pushed the S_invlist_trim-notsomuch branch from 80dd43e to 573e4fd Compare August 10, 2025 20:11
@khwilliamson khwilliamson dismissed their stale review August 11, 2025 01:23

changed as requested

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khwilliamson khwilliamson khwilliamson left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

S_invlist_trim maybe doesn't need to SvPV_renew every time?
3 participants
@richardleach @bulk88 @khwilliamson